mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaMcpConfigDialog.kt (4)

36-42: Consider logging serialization failures.

The function silently returns "{}" on serialization errors, which may hide configuration issues during debugging. Consider logging the exception for troubleshooting purposes.

 private fun serializeMcpConfig(servers: Map<String, McpServerConfig>): String {
     return try {
         json.encodeToString(servers)
     } catch (e: Exception) {
+        println("⚠️ Failed to serialize MCP config: ${e.message}")
         "{}"
     }
 }

---

114-164: Redundant nested scope.launch in LaunchedEffect.

The inner scope.launch on line 121 is redundant since LaunchedEffect already provides a coroutine scope. This creates an extra coroutine unnecessarily.

     LaunchedEffect(Unit) {
-        scope.launch {
             try {
                 toolConfig = ConfigManager.loadToolConfig()
                 mcpConfigJson = serializeMcpConfig(toolConfig.mcpServers)
 
                 if (toolConfig.mcpServers.isNotEmpty()) {
-                    scope.launch {
+                    // No need for nested launch - already in coroutine scope
                         // Create callback for incremental loading
                         val callback = object : McpLoadingStateCallback {
                             // ... callback implementation
                         }
                         // ... rest of loading logic
-                    }
                 }
 
                 isLoading = false
             } catch (e: Exception) {
                 // ... error handling
             }
-        }
     }

---

276-294: Duplicate callback object definition.

The McpLoadingStateCallback implementation is duplicated here (lines 276-289) and in the initial loading (lines 123-140). Consider extracting to a helper function to reduce code duplication.

// Extract to a helper function outside the composable or at the top of the function:
fun createLoadingCallback(
    onStateUpdate: (McpLoadingState) -> Unit,
    onToolsUpdate: (String, List<ToolItem>) -> Unit
): McpLoadingStateCallback = object : McpLoadingStateCallback {
    override fun onServerStateChanged(serverName: String, state: McpServerState) {
        // ... implementation
    }
    override fun onLoadingStateChanged(loadingState: McpLoadingState) {
        onStateUpdate(loadingState)
    }
    override fun onBuiltinToolsLoaded(tools: List<ToolItem>) {
        // ... implementation
    }
}

---

416-425: JSON editor lacks visual styling.

The BasicTextField for MCP server JSON configuration lacks visual boundaries (e.g., background, border), making it hard to distinguish from surrounding content and potentially confusing for users.

         BasicTextField(
             state = textFieldState,
-            modifier = Modifier.fillMaxWidth().weight(1f),
+            modifier = Modifier
+                .fillMaxWidth()
+                .weight(1f)
+                .background(JewelTheme.globalColors.panelBackground)
+                .padding(8.dp),
             textStyle = TextStyle(
                 fontFamily = androidx.compose.ui.text.font.FontFamily.Monospace,
                 color = org.jetbrains.jewel.foundation.theme.JewelTheme.globalColors.text.normal
             ),
             cursorBrush = SolidColor(org.jetbrains.jewel.foundation.theme.JewelTheme.globalColors.text.normal)
         )

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaPromptOptimizationDialog.kt (2)

43-43: Unused variable scope.

The rememberCoroutineScope() result is assigned to scope but never used, as LaunchedEffect provides its own scope.

-    val scope = rememberCoroutineScope()

---

108-118: Inconsistent BasicTextField API usage.

The original text field uses the value/onValueChange pattern while the enhanced text field uses the state pattern. For consistency and future maintainability, consider using the same approach for both.

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInEditorInput.kt (2)

110-110: Inline fully-qualified class name should be imported.

cc.unitmesh.indexer.DomainDictService is referenced inline. Consider adding it to the imports for consistency and readability.

+import cc.unitmesh.indexer.DomainDictService
 // ...
-val domainDictService = cc.unitmesh.indexer.DomainDictService(fileSystem)
+val domainDictService = DomainDictService(fileSystem)

---

104-112: Non-null assertion after null check could use safer Kotlin idioms.

Using currentModelConfig!! after checking currentModelConfig != null works but is less idiomatic. Consider using let or a local variable.

-                if (currentModelConfig != null && currentModelConfig!!.isValid()) {
-                    llmService = KoogLLMService.create(currentModelConfig!!)
+                currentModelConfig?.takeIf { it.isValid() }?.let { config ->
+                    llmService = KoogLLMService.create(config)
 
                     val workspace = WorkspaceManager.currentWorkspace
                     if (workspace != null) {
                         val fileSystem = workspace.fileSystem
-                        val domainDictService = cc.unitmesh.indexer.DomainDictService(fileSystem)
-                        promptEnhancer = PromptEnhancer(llmService!!, fileSystem, domainDictService)
+                        val domainDictService = DomainDictService(fileSystem)
+                        promptEnhancer = PromptEnhancer(llmService!!, fileSystem, domainDictService)
                     }
                 }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInEditorInput.kt (3)

63-73: Ensure IdeaDevInInput is properly disposed to avoid leaks.

inputComponent is created and remembered but never explicitly tied to the passed disposable. If IdeaDevInInput registers document listeners / message-bus subscriptions, this risks leaks when the parent is disposed.

Consider registering it with the parent disposable (or otherwise wiring its lifecycle) if it implements Disposable, e.g.:

val inputComponent = remember {
    IdeaDevInInput(
        project = project,
        listeners = emptyList(),
        disposable = disposable,
        showAgent = true
    ).apply {
        text = initialText
        // If IdeaDevInInput is Disposable:
        // Disposer.register(disposable, this)
    }
}

---

75-97: Send button enablement is not reactive to text changes.

sendEnabled = inputComponent.text.isNotBlank() is evaluated during composition, but Compose isn’t observing inputComponent.text, so typing in the Swing editor won’t automatically recompose and update the enabled state.

Track the text in Compose state and update it from IdeaInputListener.onTextChanged:

var inputText by remember { mutableStateOf(initialText) }

DisposableEffect(Unit) {
    val listener = object : IdeaInputListener {
        override fun onSubmit(text: String, trigger: IdeaInputTrigger) {
            onSubmit(text)
            inputComponent.clearInput()
            inputText = "" // keep state in sync
        }

        override fun onTextChanged(text: String) {
            inputText = text
        }

        override fun editorAdded(editor: com.intellij.openapi.editor.ex.EditorEx) { }
    }

    inputComponent.addInputListener(listener)
    onDispose { inputComponent.removeInputListener(listener) }
}

IdeaBottomToolbar(
    onSendClick = {
        val text = inputComponent.text.trim()
        if (text.isNotEmpty()) {
            onSubmit(text)
            inputComponent.clearInput()
            inputText = ""
        }
    },
    sendEnabled = inputText.isNotBlank(),
    // ...
)

Ensure programmatic updates (e.g., from prompt optimization) also drive inputText via onTextChanged or explicit state updates.

Also applies to: 150-160

---

192-197: Model config dialog flag is immediately reset, so UI never appears.

When showModelConfigDialog becomes true, this branch runs once and immediately sets it back to false, with no UI. From the user’s perspective, clicking “Configure” appears to do nothing.

If the real dialog isn’t ready yet, show a minimal placeholder dialog or disable the configure action until implemented. For example:

if (showModelConfigDialog) {
    Dialog(onDismissRequest = { showModelConfigDialog = false }) {
        Column(Modifier.padding(16.dp)) {
            Text("Model configuration is not yet available in this build.")
            Spacer(Modifier.height(16.dp))
            DefaultButton(onClick = { showModelConfigDialog = false }) {
                Text("OK")
            }
        }
    }
}

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaModelSelector.kt (2)

46-73: Hover interaction wiring looks solid; consider sharing interactionSource with clickable

The hover-based background and expanded coupling are clean and keep the selector visually lightweight. One small improvement: you can pass the same interactionSource into clickable so hover/press states share a single source and you avoid creating an extra one under the hood.

For example:

Row(
    modifier = Modifier
        .clip(RoundedCornerShape(6.dp))
        .hoverable(interactionSource = interactionSource)
        .background(
            if (isHovered || expanded)
                JewelTheme.globalColors.borders.normal.copy(alpha = 0.3f)
            else
                Color.Transparent
        )
        .clickable(
            interactionSource = interactionSource,
            indication = null, // or keep default if you want ripple
        ) { expanded = true }
        .padding(horizontal = 8.dp, vertical = 6.dp),
    ...
)

Purely optional, but keeps interaction state centralized.

---

74-93: Handle long model names more gracefully in the selector label

The inline label composition looks good, but with maxLines = 1 and no width/overflow control, very long model names may either stretch the toolbar or get hard-clipped.

You might want to constrain and ellipsize the text, for example:

Text(
    text = displayText,
    style = JewelTheme.defaultTextStyle.copy(
        fontSize = 12.sp,
        color = JewelTheme.globalColors.text.normal
    ),
    maxLines = 1,
    modifier = Modifier.weight(1f),
    overflow = TextOverflow.Ellipsis,
)

This keeps the selector compact while still signaling truncated content.

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaBottomToolbar.kt (3)

24-30: KDoc matches the new layout; consider slightly clarifying the right‑side description

The KDoc correctly documents the new layout, but the bullet “Right side: MCP and prompt optimization” omits the @, /, and send/stop actions.

If you want it to be fully explicit, a minor tweak like this would help:

- * - Right side: MCP and prompt optimization
+ * - Right side: action triggers (@, /), MCP settings, prompt optimization, send/stop

---

49-80: Left‑side layout and token indicator look solid; consider making the token display a bit clearer

The model selector integration and subtle token text ("${totalTokens}t") are implemented cleanly and match the described layout.

If you expect non‑expert users, you might consider a slightly clearer label or a tooltip (e.g., “123 tokens”) instead of the bare "123t", but this is purely UX polish.

---

82-139: Right‑side actions and prompt optimization button are wired correctly; optional enable/disable control

The right‑side cluster wiring (MCP settings + new prompt optimization button) looks correct, with appropriate contentDescriptions and consistent icon sizing.

If there are cases where prompt optimization should be disabled (e.g., empty input or optimization already running), you might want to add a dedicated promptOptimizationEnabled flag, similar to sendEnabled, and thread it through:

 fun IdeaBottomToolbar(
     onSendClick: () -> Unit,
     sendEnabled: Boolean,
@@
-    onSettingsClick: () -> Unit = {},
-    onPromptOptimizationClick: () -> Unit = {},
+    onSettingsClick: () -> Unit = {},
+    onPromptOptimizationClick: () -> Unit = {},
+    promptOptimizationEnabled: Boolean = true,
@@
-            IconButton(
-                onClick = onPromptOptimizationClick,
-                modifier = Modifier.size(32.dp)
-            ) {
+            IconButton(
+                onClick = onPromptOptimizationClick,
+                enabled = promptOptimizationEnabled,
+                modifier = Modifier.size(32.dp)
+            ) {

Totally optional, depending on how the dialog is used.

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInEditorInput.kt (2)

37-48: initialText / placeholder are effectively write‑only props.

initialText is applied once in the remember block, and placeholder is currently unused. If callers expect these parameters to be honored when they change over time (e.g., loading a draft or showing contextual placeholders), the current wiring won’t update the Swing component.

If dynamic updates are desired, consider:

• Passing placeholder through to IdeaDevInInput (if it supports it), and
• Syncing initialText via a side effect keyed on it:

LaunchedEffect(initialText) {
    if (inputComponent.text != initialText) {
        inputComponent.text = initialText
    }
}

Otherwise, consider documenting that they are only used at first composition or trimming unused parameters.

Also applies to: 63-73

---

206-217: Guard prompt optimization when no enhancer is available.

IdeaPromptOptimizationDialog is always opened when showPromptOptimizationDialog is true, with enhancer = promptEnhancer. If promptEnhancer is null (e.g., config load failed, invalid model, or no workspace), the dialog implementation must handle that gracefully.

If IdeaPromptOptimizationDialog does not support a null enhancer, consider:

• Disabling or hiding the prompt optimization button until promptEnhancer is non-null, or
• Showing a message in place of optimization (e.g., “Configure a valid model to enable prompt optimization”).

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInEditorInput.kt (3)

64-73: Register inputComponent with parent disposable to prevent leaks.

The inputComponent is created but not explicitly registered with the parent disposable, which could lead to resource leaks if IdeaDevInInput holds document listeners or message bus connections.

Consider registering with:

val inputComponent = remember {
    IdeaDevInInput(
        project = project,
        listeners = emptyList(),
        disposable = disposable,
        showAgent = true
    ).apply {
        text = initialText
        Disposer.register(disposable, this)
    }
}

Based on past review comment.

---

184-184: sendEnabled won't update reactively when user types.

inputComponent.text.isNotBlank() is evaluated at composition time but won't trigger recomposition when the Swing text changes. The Send button's enabled state will remain stale.

Track the input text in Compose state and update it from the IdeaInputListener.onTextChanged callback:

+    var inputText by remember { mutableStateOf(initialText) }
+
     // Add submit listener
     DisposableEffect(Unit) {
         val listener = object : IdeaInputListener {
             // ...
             override fun onTextChanged(text: String) {
-                // Handle text changes if needed
+                inputText = text
             }
             // ...
         }
         // ...
     }
     
     // In IdeaBottomToolbar:
-    sendEnabled = inputComponent.text.isNotBlank(),
+    sendEnabled = inputText.isNotBlank(),

Based on past review comment.

---

204-208: Model configuration dialog immediately dismisses.

The TODO placeholder sets showModelConfigDialog = false immediately, so clicking "Configure Model..." has no visible effect.

Consider showing a placeholder message or temporarily disabling the configure option:

     if (showModelConfigDialog) {
-        // TODO: Create IDEA version of model configuration dialog
-        // For now, just close it
-        showModelConfigDialog = false
+        org.jetbrains.jewel.ui.component.Dialog(
+            onDismissRequest = { showModelConfigDialog = false }
+        ) {
+            Column(modifier = Modifier.padding(16.dp)) {
+                Text("Model Configuration")
+                Spacer(modifier = Modifier.height(8.dp))
+                Text("Coming soon - use Settings to configure models")
+                Spacer(modifier = Modifier.height(16.dp))
+                org.jetbrains.jewel.ui.component.DefaultButton(
+                    onClick = { showModelConfigDialog = false }
+                ) {
+                    Text("OK")
+                }
+            }
+        }
     }

Based on past review comment.

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaDevInEditorInput.kt (2)

108-118: Prefer idiomatic null handling over !! operator.

The null check followed by !! is safe but not idiomatic. Consider using ?.let for cleaner, more Kotlin-style code.

-                // Initialize LLM service and prompt enhancer
-                if (currentModelConfig != null && currentModelConfig!!.isValid()) {
-                    llmService = KoogLLMService.create(currentModelConfig!!)
-
-                    val workspace = WorkspaceManager.currentWorkspace
-                    if (workspace != null) {
-                        val fileSystem = workspace.fileSystem
-                        val domainDictService = cc.unitmesh.indexer.DomainDictService(fileSystem)
-                        promptEnhancer = PromptEnhancer(llmService!!, fileSystem, domainDictService)
-                    }
-                }
+                // Initialize LLM service and prompt enhancer
+                currentModelConfig?.takeIf { it.isValid() }?.let { config ->
+                    llmService = KoogLLMService.create(config)
+
+                    WorkspaceManager.currentWorkspace?.let { workspace ->
+                        val fileSystem = workspace.fileSystem
+                        val domainDictService = cc.unitmesh.indexer.DomainDictService(fileSystem)
+                        promptEnhancer = PromptEnhancer(llmService!!, fileSystem, domainDictService)
+                    }
+                }

---

141-154: Toolbar actions have placeholder implementations.

Several toolbar actions (clipboard paste, save to workspace, insert current selection, file picker) are marked with TODO comments and not yet implemented.

These incomplete features may affect user experience. Do you want me to open an issue to track implementing these actions, or would you like help generating the implementation code?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)

297-304: Remove unused parameters workspacePath and onAtClick.

These parameters are declared but never used within the function body. If the functionality they represent has been intentionally removed (as suggested by the AI summary regarding disconnected shortcuts), the parameters should be cleaned up to avoid confusion.

 private fun IdeaDevInInputArea(
     project: Project,
     parentDisposable: Disposable,
     isProcessing: Boolean,
     onSend: (String) -> Unit,
     onAbort: () -> Unit,
-    workspacePath: String? = null,
     totalTokens: Int? = null,
     onSettingsClick: () -> Unit = {},
-    onAtClick: () -> Unit = {},
     availableConfigs: List<NamedModelConfig> = emptyList(),
     currentConfigName: String? = null,
     onConfigSelect: (NamedModelConfig) -> Unit = {},
     onConfigureClick: () -> Unit = {}
 )

Also update the call sites at lines 174, 177, 220, and 223 to remove these arguments.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/toolwindow/IdeaAgentApp.kt (1)

151-251: Split-pane layout is solid; consider gating REMOTE sends on connectivity

The new IdeaVerticalResizableSplitPane usage for CODING/LOCAL_CHAT and REMOTE reads clean and matches the existing split-pane patterns (e.g., code review). The timeline/input separation, ratios, and weight usage look good.

One improvement for the REMOTE branch: you already collect remoteIsConnected but do not use it when sending. Today the user can submit tasks while disconnected and only gets a “project or Git URL” error, not a connectivity hint.

You could gate sending on connectivity and surface a clearer message:

             IdeaDevInInputArea(
                 project = project,
                 parentDisposable = viewModel,
                 isProcessing = remoteIsExecuting,
                 onSend = { task ->
-                    val effectiveProjectId = getEffectiveProjectId(remoteProjectId, remoteGitUrl)
-                    if (effectiveProjectId.isNotBlank()) {
-                        remoteVm.executeTask(effectiveProjectId, task, remoteGitUrl)
-                    } else {
-                        remoteVm.renderer.renderError("Please provide a project or Git URL")
-                    }
+                    if (!remoteIsConnected) {
+                        remoteVm.renderer.renderError("Please connect to the remote agent server first")
+                        return@IdeaDevInInputArea
+                    }
+
+                    val effectiveProjectId = getEffectiveProjectId(remoteProjectId, remoteGitUrl)
+                    if (effectiveProjectId.isNotBlank()) {
+                        remoteVm.executeTask(effectiveProjectId, task, remoteGitUrl)
+                    } else {
+                        remoteVm.renderer.renderError("Please provide a project ID or Git URL")
+                    }
                 },

This keeps the new layout intact while tightening the REMOTE UX.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaModelSelector.kt (4)

20-20: Redundant import alias.

Line 13 already imports androidx.compose.ui.Alignment, and line 20 creates an alias ComposeAlignment for the same type. This alias is only used once (line 100), while the unqualified Alignment is used elsewhere (lines 72, 185). Remove the alias and use the unqualified Alignment throughout for consistency.

Apply this diff to remove the redundant alias:

-import androidx.compose.ui.Alignment as ComposeAlignment

Then update line 100:

-                alignment = ComposeAlignment.BottomStart,
+                alignment = Alignment.BottomStart,

---

64-69: Consider using background color tokens instead of border color.

Using JewelTheme.globalColors.borders.normal for the hover background is semantically incorrect. Border colors are typically distinct from background/overlay colors in design systems. Consider using an appropriate background or overlay color token to ensure the component remains theme-consistent.

Also, simplify the transparent color on line 68:

-                        androidx.compose.ui.graphics.Color.Transparent
+                        Color.Transparent

---

62-94: Consider extracting design values into theme tokens.

Multiple hardcoded values appear throughout the component (sizes: 6.dp, 8.dp, 4.dp, 14.dp; alphas: 0.7f, 0.6f; fontSize: 12.sp). While functional, extracting these into theme tokens or constants would improve maintainability and design consistency across the codebase.

---

75-80: Consider adding content description for the SmartToy icon.

The SmartToy icon provides semantic meaning (AI/model indicator) but has contentDescription = null. While the adjacent text provides context, adding a description like "Model" would improve accessibility for screen reader users.

             Icon(
                 imageVector = IdeaComposeIcons.SmartToy,
-                contentDescription = null,
+                contentDescription = "Model",
                 tint = JewelTheme.globalColors.text.normal.copy(alpha = 0.7f),
                 modifier = Modifier.size(14.dp)
             )

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: phodal/auto-dev-sketch PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-30T02:30:49.805Z
Learning: Applies to **/src/{androidMain,desktopMain}/**/*.kt : For Compose (Desktop/Android), use `AutoDevColors` from `cc.unitmesh.devins.ui.compose.theme` or `MaterialTheme.colorScheme`